-
Notifications
You must be signed in to change notification settings - Fork 36
Add new interface for adding alive test methods #329
Conversation
2876c6b
to
db696e1
Compare
Instead of using a bit flag which entails all alive tests we can now also use xml elements for every alive test method.
db696e1
to
a084038
Compare
Codecov Report
@@ Coverage Diff @@
## master #329 +/- ##
==========================================
+ Coverage 73.52% 73.74% +0.21%
==========================================
Files 23 23
Lines 2531 2552 +21
==========================================
+ Hits 1861 1882 +21
Misses 670 670
Continue to review full report at Codecov.
|
ospd/protocol.py
Outdated
@@ -146,6 +146,32 @@ def process_credentials_elements(cred_tree: Element) -> Dict: | |||
|
|||
return credentials | |||
|
|||
@staticmethod | |||
def process_alive_test_methods(cred_tree: Element, options: Dict) -> None: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe a more descriptive param name.
def process_alive_test_methods(cred_tree: Element, options: Dict) -> None: | |
def process_alive_test_methods(alive_test_tree: Element, options: Dict) -> None: |
</alive_test_methods> | ||
""" | ||
for child in cred_tree: | ||
if child.tag == 'icmp': |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are these methods optional? shouldn't be added only if they are set? Not sure if it make sense to add and empty element to the options dict.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All the options (imcp, tcp_ack ...) are optional. For example if you do not want icmp then you can either leave it out completely, set it to '0' or leave it empty. Everything not '1' is treated as not wanted.
Should I check if child.text is None and not include it in the options dict?
Just an additional remark. If <alive_test> and <alive_test_methods> is provided, <alive_test> takes precedence over <alive_test_methods>.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have now added a test for None. So if there is no text it will not be included options dict.
I also changed the argument description in the function.
Could you add a test ? in tests/test_scan_and_results.py there are already some tests for target options. |
Test for all available methods. Test for empty or missing methods.
What:
Instead of using a bit field which entails all alive tests we can now also use xml elements for every alive test method.
Depends on:
greenbone/ospd-openvas#331
Why:
Better usability.
How:
Tested with python-gvm and following target description with different configurations.
Checklist: